refactor: deduplicate accept loop, slowlog, pubsub in ember-server - #98
Merged
Conversation
…rver.rs run() and run_concurrent() duplicated ~200 lines of identical accept logic across 4 select arms. extracted three helpers: - accept_connection: sets nodelay, acquires semaphore, updates metrics - tls_handshake: TLS handshake with 10s timeout - on_connection_done: decrements active connections, records metrics - drain_connections: shared shutdown drain with 30s timeout each select arm now reads as a short pipeline instead of a wall of inline bookkeeping.
the lock-with-poison-recovery pattern appeared 4 times with two variants (clear-on-poison for writes vs recover-silently for reads). extracted lock_or_clear() and lock_inner() — each call site is now a single line.
subscribe/psubscribe and unsubscribe/punsubscribe differed only in which DashMap they operated on. extracted subscribe_to() and unsubscribe_from() — each public method is now a one-liner delegating to the shared helper with the appropriate map.
kacy
added a commit
that referenced
this pull request
Feb 19, 2026
* refactor: extract accept-connection and tls-handshake helpers from server.rs run() and run_concurrent() duplicated ~200 lines of identical accept logic across 4 select arms. extracted three helpers: - accept_connection: sets nodelay, acquires semaphore, updates metrics - tls_handshake: TLS handshake with 10s timeout - on_connection_done: decrements active connections, records metrics - drain_connections: shared shutdown drain with 30s timeout each select arm now reads as a short pipeline instead of a wall of inline bookkeeping. * refactor: extract slowlog mutex helpers the lock-with-poison-recovery pattern appeared 4 times with two variants (clear-on-poison for writes vs recover-silently for reads). extracted lock_or_clear() and lock_inner() — each call site is now a single line. * refactor: deduplicate pubsub subscribe/unsubscribe subscribe/psubscribe and unsubscribe/punsubscribe differed only in which DashMap they operated on. extracted subscribe_to() and unsubscribe_from() — each public method is now a one-liner delegating to the shared helper with the appropriate map.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
summary
accept_connection,tls_handshake,on_connection_done, anddrain_connectionshelpers fromserver.rs, reducing the 4 accept-loop arms from ~25 lines each to ~8 lineslock_or_clearandlock_innermutex helpers inslowlog.rs, replacing 4 inline lock-with-poison-recovery blockssubscribe_toandunsubscribe_frominpubsub.rs, deduplicating subscribe/psubscribe and unsubscribe/punsubscribewhat was tested
cargo clippy -p ember-server -- -D warnings— cleancargo test -p ember-server— all 43 tests passdesign considerations
the
accept_connectionhelper intentionally does not handle protected mode rejection — that requires writing to the owned stream, which the helper doesn't own. keeping the 3-line protected mode check in the caller preserves clarity about who owns the stream at each point.